Yep. Removing would break undici.

can't you just depend on llhttp? it's kept in a separate repo and everything.

The approach for undici then should be to open a PR that exposes a public API for interacting with the http parser. Using process.binding() directly is not supported and should not be used.

(or what @devsnek said)

While +1 in general, process.binding('constants') still seems used in popular packages, let's patch those. Also, it seems safe.

Perhaps we could move forward by runtime-deprecating everything except a list of some exceptions (for undici / constants / other usecases that we would find)? That would still improve the situation and perhaps should be much easier to get consensus on?

That is, if we don't resolve those by the time of the next major release.

Upd: will recheck constants in a bit, that could already be fixed (that was used in older Emscripten-generated code, could be fixed now there, or not).

can't you just depend on llhttp? it's kept in a separate repo and everything.

We don’t want native deps. What we would like is to have a web assembly build of llhttp. But we need help for that...

Also there is the consume/unconsume optimization which I’m not sure is possible without process.binding.

In that case we would have to bring undici into core or somehow expose that functionality.

we would have to bring undici into core

This would be a reasonable move, IMO

we would have to bring undici into core

This would be a reasonable move, IMO

The biggest task with that is to port all the tests from tap.

I’m actually sceptical to whether bringing undici into core give any value other than:

1. Get rid of process.binding hack
2. Maybe encourage more usage.
3. Maybe get more collaborators involved.

Doing it just for 1 seems a bit inefficient in terms of time and effort.

I would add:

4. A better, more modern and faster builtin API for HTTP requests.

So could this be a way forward?

1. Runtime deprecate process.binding but add an exception for undici?
2. Look into moving undici into core.

I'm happy with having undici both inside and outside of core. My main concern is the tests which I don't quite know what to do about. Would it be acceptable to have tap based tests in core?

I would not be against adding tap in the repo for tests. Happy to look into it. We are talking about https://github.com/tapjs/node-tap, right?

I'd rather not carve out any special cases as it just makes things more difficult. The viable alternatives that I can see are either: a) bring undici into core, b) expose the http parser as a supported core module, c) have undici depend on llhttp directly. Given that I believe there are several libraries that are getting to the http parser the same way, option (b) may actually be the better way to go.

@ronag i think by default the build output is just plain js, but i could be wrong cc @indutny

@ronag i think by default the build output is just plain js, but i could be wrong cc @indutny

I think we lose performance that way.

Oh, just remembered... it is also possible to get to the internal HTTPParser via require('_http_common')

root@DESKTOP-5KK9VIR:~/node/node# ./node -pe "require('_http_common').HTTPParser"
[Function: HTTPParser] {
  REQUEST: 1,
  RESPONSE: 2,
  kOnMessageBegin: 0,
  kOnHeaders: 1,
  kOnHeadersComplete: 2,
  kOnBody: 3,
  kOnMessageComplete: 4,
  kOnExecute: 5,
  kOnTimeout: 6
}

That would likely be the most immediate way of avoiding the direct use of process.binding(). It's still not a supported public API so may still run into issues down the road, but it addresses the most immediate concern

wasm seems to work at least 😄

How do I do that?

https://github.com/devsnek/llhttp/tree/wasm

Could we get the wasm build on npm?

I'm not planning to do so but I think as an org yes that is possible. We already publish readable-stream, for example.

llhttp is already on npm. Though a bit outdated and lacking wasm.

This is probably going to annoy many people but maybe that's necessary so the ecosystem migrates out of it.
For example, node-core-utils currently prints a pending deprecation warning because of https://www.npmjs.com/package/external-editor, which depends on an old version of https://www.npmjs.com/package/tmp which uses process.binding.

And yet tmp was patched to avoid that back in 2018 (raszi/node-tmp@f2609c9)

And yet tmp was patched to avoid that back in 2018 (raszi/node-tmp@f2609c9)

This is a common theme, as you probably are aware. In the current download stats (screenshot below), you can see that the current [email protected] has an impressive 4 million downloads in the last 7 days...but it's a fraction of the nearly 13 million downloads achieved in the last 7 days by 0.0.33 published 4 years ago.

Yep... thus the reason why I said "It's time to start nudging them harder out of the nest." in the original post ;-)

@nodejs/tsc ... this needs additional review please.

CI: https://ci.nodejs.org/job/node-test-pull-request/36384/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2635/

I found following uses in my code:

• process.binding("natives"): I use only the keys. Most likely I can replace it by module.builtinModules but the difference is that it doesn't include internal modules
• process.binding("pipe_wrap").Pipe: I use socket._handle instanceof Pipe to check if a socket is connected to a Pipe. Can't remember why I did it that way but I assume I will find a different approach.

Closing this in favor of a more incremental approach